-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add applymud
subcommand
#196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -158,7 +159,7 @@ def _define_arguments(): | |||
return args | |||
|
|||
|
|||
def _add_mud_selection_group(p, is_gui=False): | |||
def _add_mud_selection_group(p, use_gui=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed is_gui
to use_gui
because I feel like it's a better variable name
def get_args(override_cli_inputs=None): | ||
p = ArgumentParser() | ||
p = _add_mud_selection_group(p, is_gui=False) | ||
def _register_applymud_subparser(subp, use_gui=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it's basically the same as before, but I'm taking it out as a separate function for adding arguments to applymud
. I think it'll be cleaner when define another one for getmud
later.
for arg in _define_arguments(): | ||
kwargs = {key: value for key, value in arg.items() if key != "name"} | ||
p.add_argument(*arg["name"], **kwargs) | ||
def create_parser(use_gui=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function for creating gooey/argument parser
@@ -45,6 +45,7 @@ | |||
"wavelength", | |||
"theoretical_from_density", | |||
"theoretical_from_packing", | |||
"subcommand", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not write subcommand in the output files
@@ -115,7 +115,7 @@ def test_set_input_lists(inputs, expected, user_filesystem): | |||
base_dir.resolve() / expected_path for expected_path in expected | |||
] | |||
|
|||
cli_inputs = inputs + ["--mud", "2.5"] | |||
cli_inputs = ["applymud"] + inputs + ["--mud", "2.5"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here just adding applymud to cli inputs for all tests
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #196 +/- ##
=======================================
Coverage 99.31% 99.31%
=======================================
Files 5 5
Lines 292 292
=======================================
Hits 290 290
Misses 2 2
🚀 New features to boost your workflow:
|
This looks good. I recommend we work a bit on the argparse helps so they are more readable when they are in the gui. Do we have docs? Some of the longer explanations can go to the docs if we have them. Or is there a way to have them longerin in the cli but shorter in the gui? That could work too. For the top input, in the gui it is not so helpful a name for the variable. maybe |
Sounds great! I opened a new issue for this #197 |
closes #191
The
getmud
subcommand is tracked through #193 and #181. Docs update for this PR is tracked through #192.Here're screenshots of the GUI interface:


@sbillinge ready for review